-
Notifications
You must be signed in to change notification settings - Fork 24
core: Add STACKIT CLI Auth flow #2179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Jan-Otto Kröpke <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution.
I left a comment regarding the GitHub Workflow dependency
Signed-off-by: Jan-Otto Kröpke <[email protected]>
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
marceljk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good and works as expected. 😃 Can you just update the date of the changelog?
Co-authored-by: Marcel Jacek <[email protected]>
Co-authored-by: Marcel Jacek <[email protected]>
|
@marceljk Done and thanks for the additional testing. Keep in mind to set |
|
@jkroepke We just discussed the field |
|
Hi @marceljk I would have a different sight on that: On the Azure SDK, it's enabled by default. This at least guarantees that CLI authentication is always available, even for third-party programs like Prometheus, Terraform. And it's documented: The AWS SDK works in a similar way. An authenticated CLI is sufficient for the Terraform provider. That is great for local development where I can use a personal account to run terraform locally. From an end-user perspective, I would assume that CLI authentication is enabled by default. So I wound recommend that CLI Auth Flow is part of the Standard Credential Chain. But I respect your decision and have changed it as requested. |
Signed-off-by: Jan-Otto Kröpke <[email protected]>
2fd910a to
60aea17
Compare
|
Hi @jkroepke, that's a good point that the CLI authentication is by default always available and even for third-party programs. We will discuss it again internal, what's the better option here and will come back to you. |
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
|
Hi @jkroepke, |
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
|
Hello @jkroepke, Thanks for your contribution! We really appreciated your contribution and this gave us some new input for our internal discussion and how we can implement this feature. We created a RFC stackitcloud/terraform-provider-stackit#880 with our target architecture of this feature. We would appreciate it, when you give us some feedback to it. When we implement this feature, we can also add you as reviewer if you like to. With this newly defined architecture, I will close this PR. |
Description
relates to stackitcloud/terraform-provider-stackit#719
This PR adds the ability to obtain a token from a locally installed CLI. This is great for local development, where a real user is logged in via the CLI, and other STACKIT-related tools like the Terraform provider can use the token from the authenticated CLI context to make additional requests against the STACKIT API.
While developing this feature, I found stackitcloud/stackit-cli#736.
Due limited STACKIT access, tests with real human accounts can not be done by myself.
The Stackit CLI flow is enabled by default. There is an option to disable the cli flow. On CLI itself, the option should set to false to prevent infinite loops.
The path of the STACKIT CLI is not configurable. The SDK expects the CLI to be available within the system PATH. I modified the Github Action to ensure that the Stackit CLI is present.
Tests are using a custom CLI profile to avoid interrupting the local default configuration.
Code inspired from https://github.com/Azure/azure-sdk-for-go/blob/f4b3a417e0bc39a5051028c4f9d0fc36bc612317/sdk/azidentity/azure_cli_credential.go#L119-L166
Checklist
make fmtexamples/directory)make test(will be checked by CI)make lint(will be checked by CI)